fix(llm): honour GRAPHIFY_API_TIMEOUT in the claude-cli backend, not just HTTP - #1111
Closed
yaanfpv wants to merge 1 commit into
Closed
fix(llm): honour GRAPHIFY_API_TIMEOUT in the claude-cli backend, not just HTTP#1111yaanfpv wants to merge 1 commit into
yaanfpv wants to merge 1 commit into
Conversation
…just HTTP GRAPHIFY_API_TIMEOUT (and the --api-timeout flag that sets it) bounded only the OpenAI-compatible HTTP client. The two claude-cli subprocess calls, extraction and community labeling, hardcoded timeout=600 and ignored the knob entirely, so a slow chunk on claude-cli could not be given more headroom without patching the source. Factor the parse the HTTP path already used into a _resolve_api_timeout helper (mirroring _resolve_max_tokens) and route all three call sites through it. Behaviour for the HTTP backend is unchanged. The claude-cli extraction and labeling subprocesses now respect the same env var and flag, defaulting to 600s when unset and falling back to the default on non-positive or unparseable values.
Collaborator
|
Implemented from our end in 75c4de7 with two additions over the PR: (1) the Anthropic SDK path (_call_anthropic) now also honours the timeout, so the README claim 'every backend' is accurate; (2) added a test for the _call_llm claude-cli branch separately from _call_claude_cli. Thanks for the diagnosis — your root cause analysis was exactly right. |
This was referenced Jul 29, 2026
safishamsi
pushed a commit
that referenced
this pull request
Jul 29, 2026
The two bedrock-runtime clients (primary extraction in _call_bedrock and the secondary dispatch path in _call_llm) were built with no botocore config, so Converse used botocore's 60s default read timeout and ignored GRAPHIFY_API_TIMEOUT / --api-timeout entirely. A long opus-class generation then died with "Read timeout on endpoint URL" no matter how high the timeout was set. Both client constructions now pass a botocore.config.Config wiring read_timeout to _resolve_api_timeout() (default 600s), a 10s connect_timeout, and retries from _resolve_max_retries() in adaptive mode. This mirrors the fixes that closed the same gap for the claude-cli subprocess (#1112/#1111) and the secondary LLM dispatch path (#1442) -- bedrock was the last cloud backend still ignoring the knob. Also updates the README env-var row, which listed the timeout as applying to HTTP/claude-cli/Anthropic only, and the _fake_boto3 test fixture to register botocore.config and capture the client config so the new coverage can assert the timeout is wired.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1112.
GRAPHIFY_API_TIMEOUT, and the--api-timeoutflag that sets it, only ever reached the OpenAI-compatible HTTP client. The twoclaude-clisubprocess calls, extraction (_call_claude_cli) and community labeling (theclaude-clibranch of_call_llm), hardcodedtimeout=600and ignored the knob completely.That bites on long
claude-cliruns. A dense chunk or a large 1M-context model can legitimately run past ten minutes, and today there is no way to give it more headroom without editing the source. The HTTP backends already advertise the escape hatch (the env-var table documents it, and #792 wired it in); the CLI path just never picked it up.What changed
_resolve_api_timeout()helper, placed next to and mirroring the existing_resolve_max_tokens()._call_claude_cli(extract), and theclaude-clibranch of_call_llm(labeling).The HTTP backend behaves exactly as before (same env var, same 600s default, same parsing). The
claude-cliextraction and labeling subprocesses now respectGRAPHIFY_API_TIMEOUT, and since--api-timeoutsimply exports that env, the flag now applies toclaude-clias well. Non-positive or unparseable values fall back to 600s, matching the HTTP path.Tests
Added to
tests/test_claude_cli_backend.py: four unit tests for_resolve_api_timeout(default, env override, invalid string, non-positive) and two that assert the resolved value reachessubprocess.run'stimeout=argument for both the default and an env override. The file passes (19 tests) andruff checkis clean.